mrdox-test supports yml configs. #112

klemens-morgenstern · 2023-04-26T14:45:28Z

This supports multiple documents per config (because yaml) so one config can create multiple runs with a single yaml file.

vinniefalco · 2023-04-26T15:15:54Z

source/test/Config.cpp

+// Copyright (c) 2023 Klemens D. Morgenstern
+//
+// Distributed under the Boost Software License, Version 1.0. (See accompanying
+// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)


It seems I have to repeat myself. We are not using the Boost license, Please follow the example set in the other files.

Sorry, IDO template

vinniefalco · 2023-04-26T15:16:11Z

source/test/Config.cpp

+}
+
+}
+}


Missing newline

vinniefalco · 2023-04-26T15:16:53Z

source/test/Config.hpp

+
+llvm::Expected<std::vector<TestConfig>>
+           loadTestConfig(llvm::StringRef dir,
+                          llvm::StringRef file);


Please follow the established style

vinniefalco · 2023-04-26T15:19:31Z

source/test/Config.cpp

+    {
+        auto fileText = llvm::MemoryBuffer::getFile(filePath);
+        if(! fileText)
+            return makeError(fileText.getError().message(), " when loading file '", filePath, "' ");


I believe you can just put fileText here instead of fileText.getError().message(). If not, we can add a nice overload for it.

vinniefalco · 2023-04-26T15:20:25Z

source/test/Config.cpp

+    clang::mrdox::TestConfig>
+{
+  static void mapping(IO &io,
+                      clang::mrdox::TestConfig& f)


It would be best if we did not have two files with the same name anywhere, we already have Config.cpp in api

vinniefalco · 2023-04-26T15:20:41Z

source/test/Config.cpp

+    if (!fs::exists(filePath))
+    {
+        filePath = dir;
+        filePath += "/mrdox-test.yml";


you have to use path::append not +=

vinniefalco · 2023-04-26T15:27:27Z

test-files/old-tests/attributes_1.yml

+cxxstd: c++20
+...
+---
+cxxstd: c++17


Hmmm... no, I don't think we should invent a new yml file format that has multiple yml configurations inside.

That's not a new yaml format, that's standard yaml.

Yes I see that but still.... where are you

vinniefalco · 2023-04-26T15:27:57Z

source/test/SingleFile.hpp

+        {
+          char buf[64];
+          snprintf(buf, 63, "-std=%s", tc.cxxstd.c_str());
+          cmds.emplace_back(buf);


please use the existing style

vinniefalco · 2023-04-26T15:30:11Z

source/test/Config.cpp

+            return makeError(fileText.getError().message(), " when loading file '", filePath, "' ");
+        llvm::yaml::Input yin(**fileText);
+
+        std::vector<TestConfig> res;


There should only be one config in the yml file. That is, given the files "1.cpp" and "1.xml", if there is a file named "1.yml" then it should contain a single config and be loaded and applied to the test instead of the defaulted config. If "1.yml" does not exist, then if there is a filed called "config.yml" located in the same directory then that should be loaded and applied to the test (you can load it once when iterating the directory and re-use it for all tests in that dir and its children).

Otherwise, if there is no 1.yml, and no config.yml in any of its parent directories, then it should do what it does now with a default config.

The latter part does happen. Allowing multiple configs per yml doesn't cost us much, but allows us to quickly add more tests. This is particularly helpful is a certain functionality doesn't work with a given flag OR on a standard.

An issue reporter can then just add his case for an existing test.

vinniefalco · 2023-04-27T00:00:39Z

source/test/SingleFile.hpp

@@ -15,6 +15,7 @@
 #include <string>
 #include <utility>
 #include <vector>
+#include "TestConfig.hpp"


includes should be ordered most specific -> most general

vinniefalco · 2023-04-27T13:16:58Z

source/test/TestConfig.cpp

+
+template<>
+struct llvm::yaml::MappingTraits<
+    clang::mrdox::TestConfig>


Wait a minute I think there's some confusion here, the tests are supposed to use the same YML configuration as the tool. Not a whole new YML configuration.

It's a different config object.

vinniefalco · 2023-05-02T14:01:09Z

source/test/SingleFile.hpp

-        cmds.emplace_back("-std=c++20");
+        {
+            char buf[64];
+            snprintf(buf, 63, "-std=%s", tc.cxxstd.c_str());


hmm this will create warnings on msvc.. prefer using llvm::raw_string_ostream

klemens-morgenstern · 2023-05-03T00:16:30Z

@vinnie rebased.

alandefreitas · 2024-06-24T18:47:24Z

Already supported now

klemens-morgenstern requested a review from vinniefalco April 26, 2023 14:45

vinniefalco reviewed Apr 26, 2023

View reviewed changes

source/test/Config.cpp Outdated

}

}

}

Copy link

Member

vinniefalco Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline

vinniefalco reviewed Apr 26, 2023

View reviewed changes

klemens-morgenstern force-pushed the test-config branch 2 times, most recently from bb83035 to 08fd7fb Compare April 26, 2023 23:55

vinniefalco reviewed Apr 27, 2023

View reviewed changes

vinniefalco reviewed May 2, 2023

View reviewed changes

klemens-morgenstern force-pushed the test-config branch from 08fd7fb to 812e33c Compare May 3, 2023 00:16

klemens-morgenstern force-pushed the test-config branch 3 times, most recently from 4e9e12a to 8571b1c Compare May 9, 2023 09:31

klemens-morgenstern force-pushed the test-config branch from 8571b1c to 9b398e5 Compare July 18, 2023 03:53

mrdox-test supports yml configs.

95fc313

klemens-morgenstern force-pushed the test-config branch from 9b398e5 to 95fc313 Compare July 18, 2023 04:50

alandefreitas closed this Jun 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mrdox-test supports yml configs. #112

mrdox-test supports yml configs. #112

klemens-morgenstern commented Apr 26, 2023

vinniefalco Apr 26, 2023

klemens-morgenstern Apr 26, 2023

vinniefalco Apr 26, 2023

vinniefalco Apr 26, 2023

vinniefalco Apr 26, 2023

vinniefalco Apr 26, 2023

vinniefalco Apr 26, 2023

vinniefalco Apr 26, 2023

klemens-morgenstern Apr 26, 2023

vinniefalco May 3, 2023

vinniefalco Apr 26, 2023

vinniefalco Apr 26, 2023

klemens-morgenstern Apr 26, 2023

vinniefalco Apr 27, 2023

vinniefalco Apr 27, 2023

klemens-morgenstern May 2, 2023

vinniefalco May 2, 2023

klemens-morgenstern commented May 3, 2023

alandefreitas commented Jun 24, 2024

+              }
+              }
+              }

mrdox-test supports yml configs. #112

mrdox-test supports yml configs. #112

Conversation

klemens-morgenstern commented Apr 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klemens-morgenstern commented May 3, 2023

alandefreitas commented Jun 24, 2024